Repository navigation
Conversation
waynercheung
left a comment
There was a problem hiding this comment.
[SHOULD] Please record the pre-change serialization comparison required by #7013, including the commands, fixtures and results, or add fixed regression fixtures for Endpoint, HelloMessage and BackupMessage. The current PR does not document this comparison.
[DISCUSS] Please reconcile #7013's conditional deprecation requirement with the clarification in #6921 that these are unsupported legacy types. Record the agreed classification and describe the generated-type removal and expected descriptor change in this PR's compatibility notes.
The four removed legacy message types have been unused by java-tron for years and are not supported external APIs, as clarified in #6921. Their removal therefore does not require a deprecation cycle. The removal of the generated types and the corresponding descriptor changes are intentional and can be documented in the PR’s compatibility notes. |
|
Thanks. On the serialization comparison: agreed, no additional permanent regression tests are needed for this cleanup. For the record, I ran the comparison against the merge base 6d5adc4: [SHOULD] On the classification: agreed. Please add the compatibility note to the PR description before merge, and update the last paragraph of #7013 to match. Suggested text:
[NIT] Please correct "defintion" to "definition" in the PR title. |
|
@waynercheung I have update the PR description and PR title, thanks for your suggestion. |
Thanks, I have update the of last paragraph of issue #7013. |
What does this PR do?
close #7013.
PingMessage,PongMessage,FindNeighbours, andNeighboursdefinitions fromprotocol/src/main/protos/core/Discover.proto.Endpoint, referenced byHelloMessage.from, andBackupMessage, used by backup keepalive messages, with their existing fields and options.p2p/src/main/proto/Discover.protoand align the UDP discovery examples and field descriptions with the implementation, including IPv6 addresses, network identifiers, and response timestamps.Compatibility:
The retained messages preserve their existing wire encoding. No runtime networking or consensus changes are introduced. The generated Java types org.tron.protos.Discover.PingMessage, PongMessage, FindNeighbours and Neighbours are removed, and the core/Discover.proto descriptor changes accordingly (BackupMessage moves from message index 5 to 1). Per #6921 these are unsupported legacy types, so no deprecation cycle is required. The active UDP discovery definitions are in p2p/src/main/proto/Discover.proto (org.tron.p2p.protos.Discover), which uses a different Java package and protobuf namespace, so they are not drop-in replacements.
Why are these changes required?
These definitions are redundant leftovers from the libp2p split and are no longer referenced by java-tron's runtime code. UDP discovery uses the separate p2p definitions, brought into this repository in #6992. Removing the legacy definitions reduces maintenance overhead and avoids confusion about the active discovery protocol.
Part of the cleanup tracked in #6921.